tweak(cmd): only enable clickhouse with ENV#139
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds conditional initialization of ClickHouse based on environment configuration, allowing the service to run without ClickHouse when not configured.
- Introduces an
Enabledflag to the ClickHouse client to track initialization state - Adds a helper function to determine whether ClickHouse should be initialized based on configuration
- Fixes MIME type validation to use a fixed buffer size instead of full file size
- Corrects a spelling error in an error message
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/clickhouse/client.go | Adds Enabled field and conditional initialization logic to skip ClickHouse when disabled |
| cmd/lite/lite.go | Implements conditional ClickHouse initialization based on host configuration |
| internal/http/middleware/mime.go | Changes buffer allocation from file size to fixed 3072 bytes and fixes spelling error |
| internal/http/httputil/mime.go | Updates buffer allocation to use fixed size instead of file header size |
| internal/service/file/file_service.go | Adds comments clarifying API usage context for methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| c := &Client{} | ||
| func NewClient(config *Config, enabled bool) *Client { | ||
| if !enabled { | ||
| slog.Warn("clickhosue is disabled, logging is not available") |
There was a problem hiding this comment.
Corrected spelling of 'clickhosue' to 'clickhouse'.
| slog.Warn("clickhosue is disabled, logging is not available") | |
| slog.Warn("clickhouse is disabled, logging is not available") |
cmd/lite/lite.go
Outdated
| if clickhouseHost != "" { | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
|
|
There was a problem hiding this comment.
This function can be simplified to a single return statement: return clickhouseHost != \"\"
| if clickhouseHost != "" { | |
| return true | |
| } | |
| return false | |
| } | |
| return clickhouseHost != "" | |
| } |
cmd/lite/lite.go
Outdated
| clickhouse.AutoMigrate(cmd.Context(), chConfig) | ||
| } | ||
|
|
||
| clickhouseClient := clickhouse.NewClient(chConfig, shouldInitClickhouse(chConfig.Host)) |
There was a problem hiding this comment.
The shouldInitClickhouse function is called twice with the same argument (lines 83 and 88). Consider storing the result from line 83 in the clickhouseEnabled variable and reusing it here.
| clickhouseClient := clickhouse.NewClient(chConfig, shouldInitClickhouse(chConfig.Host)) | |
| clickhouseClient := clickhouse.NewClient(chConfig, clickhouseEnabled) |
itschip
force-pushed
the
feat/enable-clickhouse
branch
from
30fa3e5 to
f110099
Compare
1 participant
No description provided.